fix(wizard): pin workflow persist to the source workspace - #134
Merged
Conversation
An in-flight checkpoint that hit a CAS conflict retried against the live workspace pointer. Switching folders during that retry merged the source workflows into the destination store. Co-authored-by: ignaciodelcano+dcl <ignaciodelcano+dcl@gmail.com>
PR Review — Loreframe StudioRisk: low Automated review from Findings
Changed files
CONTRIBUTING checklist
Posted by the repo PR review workflow. Re-runs on each push to the PR. |
Code healthQuality score: 49.2/100Higher is better. The score is a trend dashboard; the independent ratchet below remains the CI gate.
Change vs PR base: +0.0 points.
Markdown, JSON catalogs and tests are out of this table. Only Most complex functions
Trend vs baseline
Warnings
Ratchet passed. |
Owner
|
cursor review |
Author
Bugbot couldn't run — GitHub account mismatchThe GitHub account linked to your Cursor account does not match the PR author. Please ensure you're using the correct GitHub account, or run Bugbot from a team that covers this repository. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resumen ejecutivo
Qué cambia
Los checkpoints del Wizard ya no pueden guardarse en la carpeta de destino cuando el usuario cambia de workspace a mitad de un guardado en conflicto.
Para qué sirve
Evita que un workflow en curso de la carpeta A se mezcle o sobrescriba los workflows persistidos de la carpeta B.
Impacto para el usuario
Si cambia de carpeta de salida mientras un workflow del Wizard reintenta un guardado, B conserva sus propios checkpoints y A sigue recibiendo el suyo.
Riesgo
Estado
Summary
WizardWorkflowRuntime.persist()cloned the in-memory collection correctly, but CAS conflict retries calledload(this.workspace)/save(this.workspace)against the live pointer.open()rebinds that pointer immediately, so a folder switch during the retry merged the source checkpoint into the destination.wizard-workflows-v1.jsonand then replaced the in-memory collection.This is distinct from the rejected conversation-CAS bug (hocuspocus#48 / PR #122). Conversations already pin writes by workspace; workflow checkpoints did not.
Overview
Concrete trigger:
persist()hits a CAS conflict (second tab, concurrent workflow save, or overlapping persist).open('workspace-b').mergeCollections'd A's workflows into B, and saved the mix to B. A's latest state was also lost.The fix pins
targetWorkspace/openSequenceat the start ofpersist(). Retries always read/write that workspace. In-memory collection updates are applied only if the runtime still owns that open.persistAndEmitno longer throwsWorkflow does not existafter a switch.Detailed changes
Backend
No backend changes.
UI and Wizard
ui/src/features/agent/wizardWorkflowRuntime.ts: pin persist target; guard in-memory updates; tolerate emit after switch.ui/tests/wizardWorkflowRuntime.test.mjs: per-workspace mock that conflicts+holds the first save, then asserts B is untouched and A receives the running checkpoint.The regression test fails on the old pointer (
workflow-aappears in B) and passes with the pin.Data, provenance and compatibility
No schema or file-format change. Existing workflow JSON remains valid.
Files and ownership
ui/src/features/agent/wizardWorkflowRuntime.ts— persist/open race fix.ui/tests/wizardWorkflowRuntime.test.mjs— regression coverage.wizardConversationPersistence.ts) was left untouched; that path already pins writes.Validation
cd ui && npx tsx --tsconfig tsconfig.app.json --import ./tests/setupI18n.ts --test tests/wizardWorkflowRuntime.test.mjs tests/rhythmic3dWorkflow.test.mjs tests/wizardConversationPersistence.test.mjs— 33 passed, 0 failedworkflow-a) and passes after the pingit diff --check— cleanpython scripts/verify_clean_repo.py— N/A (no Python changes)python -m compileall— N/Acd ui && npm run i18n:check— N/A (no copy changes)cd ui && npm run lint -- --max-warnings=0— not rerun; edited files have no linter errorscd ui && npm run build— N/A (logic-only, covered by unit tests)Code quality
main: pending CICI and review
Coste de la tarea
Notes and limitations
save(this.workspace, snapshot)already captured the workspace argument; the corrupt write happens on the conflict retry afteropen()rebinds the pointer. The test forces that retry.isWizardConversationWriteCurrent).wgp.convert_imagepipeline, not a cross-workspace data-loss bug.Follow-up work
None required for this race. Optional later: serialize
persist()globally so two workflows cannot CAS-conflict on the same collection.Checklist